Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FTP Support #5627

Merged
merged 37 commits into from
Aug 15, 2021
Merged

FTP Support #5627

merged 37 commits into from
Aug 15, 2021

Conversation

hez2010
Copy link
Member

@hez2010 hez2010 commented Jul 30, 2021

Resolved / Related Issues
Items resolved / related issues by this PR.

Details of Changes
Add FTP support.

  • Protocol support: FTP(21), FTPS(990)
  • Credential support
  • Browse and Navigation
  • Copy file
  • Copy directory
  • Paste file
  • Pate directory
  • Delete and rename file
  • Delete and rename directory
  • Translations
  • Clean up

Out of Scope

  • Moving files
  • Copying between two FTP servers
  • Open files directly from FTP servers

TODO

  • Disable not supported features in context menu

Validation
How did you test these changes?

  • Built and ran the app
  • Tested the changes for accessibility

Screenshots (optional)
image

@yaira2 yaira2 requested a review from gave92 August 1, 2021 06:06
@hez2010
Copy link
Member Author

hez2010 commented Aug 5, 2021

I think this PR is almost ready. Needs add language resource later.

@yaira2 yaira2 requested a review from gave92 August 12, 2021 05:13
@gave92
Copy link
Member

gave92 commented Aug 14, 2021

This PR is looking very good. Only thing I think are left:

  • Add string resources for credential dialog
  • Disable not supported features in context menu -> which ones are they?

Issues:

  • Drag & drop does not work (since copy/paste works I'd expect this can be done too)
  • Copying a file from ftp to the desktop does not appear to work (explorer says "Interface not supported")

@hez2010
Copy link
Member Author

hez2010 commented Aug 15, 2021

Drag & drop does not work (since copy/paste works I'd expect this can be done too)

Drag & drop from explorer to files does work.

Copying a file from ftp to the desktop does not appear to work (explorer says "Interface not supported")

This needs the streamed file be downloaded to local first before explorer can copy from it.
You can paste a FTP file in files and then paste it again in explorer to verify this.

@gave92
Copy link
Member

gave92 commented Aug 15, 2021

Drag & drop from explorer to files does work.

Sorry, I meant the possibility to drag a file from a FTP location (inside Files) to a local folder (also in Files).. added :)

This needs the streamed file be downloaded to local first before explorer can copy from it.

Indeed, I've just seen this, annoying.

@hez2010
Copy link
Member Author

hez2010 commented Aug 15, 2021

I meant the possibility to drag a file from a FTP location (inside Files) to a local folder (also in Files).. added :)

Oops I missed this one, thanks!

Disable not supported features in context menu -> which ones are they?

Seems that the context menu is created based on extension name.

Do not show tags context menu item in FTP
Disable share toolbar button in FTP
@gave92
Copy link
Member

gave92 commented Aug 15, 2021

I've just pushed a few changes, if you approve of them I think this is ready to be merged.

@hez2010
Copy link
Member Author

hez2010 commented Aug 15, 2021

LGTM. Thank you!

I think we also need to add progress support for per file to reflect the download/upload progress after this PR is merged.

@gave92 gave92 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Aug 15, 2021
@gave92
Copy link
Member

gave92 commented Aug 15, 2021

Wish list.. no pressure 😂

  • Progress support for file transfers
  • Remember credentials
  • Double click downloads to TEMP and opens file
  • Move operation
  • Copy and create folders
  • Handle conflicts
  • Autorefresh folder for changes
  • ??

@yaira2
Copy link
Member

yaira2 commented Aug 15, 2021

@hez2010 thank you!

@yaira2 yaira2 merged commit d4cb507 into files-community:main Aug 15, 2021
@yaira2 yaira2 mentioned this pull request Aug 15, 2021
@yaira2
Copy link
Member

yaira2 commented Aug 15, 2021

@gave92 can you open a separate issue to track those?

@gave92 gave92 mentioned this pull request Aug 15, 2021
12 tasks
@hez2010 hez2010 deleted the feature/ftp branch August 15, 2021 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FTP Support
3 participants